fix: add missing static css files for WC #1385
Conversation
| import { viteStaticCopy } from 'vite-plugin-static-copy'; | ||
| import fs from 'fs'; | ||
|
|
||
| const gridMaterialCssPath = [ |
There was a problem hiding this comment.
this will handle only material light theme and will not handle dark theme as well as all bootstrap, indigo and fluent themes.
There was a problem hiding this comment.
Done. Added light or dark color for material bootstrap, indigo and fluent themes.
| const themeColors = ["light", "dark"]; | ||
| const themeTypes = ["material", "bootstrap", "indigo", "fluent"]; | ||
| const basePaths = [ | ||
| "node_modules/igniteui-webcomponents-grids/grids/themes", | ||
| "node_modules/@infragistics/igniteui-webcomponents-grids/grids/themes" | ||
| ]; | ||
| const themeFiles = themeColors.flatMap(color => | ||
| themeTypes.flatMap(theme => basePaths.map(basePath => `${basePath}/${color}/${theme}.css`)) | ||
| ).filter(fs.existsSync); |
There was a problem hiding this comment.
Yeah, nope :)
If the idea is to build out functionality to copy all themes in general, just copy them all - at best what you can do is hardcode the path, not the specific variants (material, etc, those can and will change).
Should be fine to just copy the entire node_modules/igniteui-webcomponents-grids theme folder and it'd do the same thing as this, right? Surely the static copy plugin can work with globs
Also, not sure why restricted to igniteui-webcomponents-grids as well, should include igniteui-webcomponents's themes too, no?
There was a problem hiding this comment.
Also we generally keep the package licensed maps separately so this could also work with one version of the packages and swap the node_modules path with the upgrade command. We don't put path maps in tsconfig for dual package support, so that will make it somewhat consistent.
There was a problem hiding this comment.
These changes are no longer needed, we will include these css files by importing them not by link tag.
| viteStaticCopy({ | ||
| targets: [ | ||
| { src: 'src/assets', dest: 'src' }, | ||
| { src: 'ig-theme.css', dest: '' }, |
There was a problem hiding this comment.
I understand why css from packages needs a static copy, but why the ig-theme which is part of the source. Is that the way vite instructs css to be handled, because I think this can also just be imported tp get it included.
There was a problem hiding this comment.
Wait.. that's not even it the project files? :O
Is that part for the CodeGen? Not sure it even belongs here if so, though you can argue it's functionality to allow to set global styling for all views; hint: the current styles.css is useless for that and your img, video styles and whatever missing resets we apply in other projects that are missing won't work from that either.
To that end, ig-theme can and should be in the templates with a comment explaining it sets up styles for all the views. The name also shouldn't really be ig-theme (that file actually had a different purpose initially) and should be included in all views generated by the CLI ofc.
Then you can justify the file handling in the project ;D
There was a problem hiding this comment.
Yes, ig-theme will be renamed and included in the templates folder by CodeGen
|
Also, has someone looked for some lit-specific plugins for vite that might do something similar? Assuming this is all because links in the lit markup, so perhaps there's a plugin that can let vite read and handle those or possibly through the view component styles via Edit: https://www.npmjs.com/package/rollup-plugin-lit-css looks interesting |
Lipata
added
💥 status: in-test
✅ status: resolved
and removed
❌ status: awaiting-test
💥 status: in-test
labels
Remove the step in github pages yml for adding the ig-theme, because we’ll use the import inline css syntax for the styles. The changes will be handled in the CodeGen, so no further modifications are needed here.